-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reversing Dependency: Implementing Untyped-RPC on Top of Typed-RPC (draft) #51
Conversation
- `Grpc_protobuf` is no longer eio specific
- Make use of ocaml-protoc's server implementation bundle as intended; - Adapt routeguide example to ocaml-protoc to show the differences. This results in a more complicated [Grpc.Rpc] interface that tries to better capture the common parts between ocaml-protoc and ocaml-protoc-plugin that can be used by ocaml-grpc.
This reverts commit 042efeb.
- This adds type safety to ensure the RPCs are called with the expected protocol (e.g. you cannot call a unary rpc with a server_streaming entry point, etc.). On the ocaml-protoc-plugin side, currently there are no markers for the rpc modes - this interface will permit adding them in the future without user facing changes. On the ocaml-protoc plugin, the value mode flows from the proto file definition and is checked in the user code as expected. Implementation note: There's perhaps a way to shorten the mapping of value-modes but I couldn't find one given that `Grpc` cannot depend on `Ocaml_protoc`, and thus the `Value_mode` types are not equal.
In the existing examples, the service name is separated from the package name by a dot, which I inadvertently omitted in the previous implementation. Note that as long as the service name used by a client and a server is the same, the right handler is executed, so there's some leeway in the actual choice of the convention to use. The hope is that the dot separated one is standard.
Currently this PR is built on top of #48 thus duplicating the commit chain.
|
@quernd and @tmcgilchrist I prefer this implementation over the one in #48. If you like it too, I will use this as the basis for the typed-rpc-eio implementation that I enable for code review. Please let me know what you think. Thank you! |
I ended up re-using this code organization in #55. |
In this Pull Request, I've reversed the dependency between the typed-RPC and
untyped-RPC implementations. Now, the untyped-RPC is built on top of the
typed-RPC, a shift from the previous structure.
This change is motivated by several factors:
It paves the way for a straightforward deprecation of the untyped-RPC in the
future, as we can simply remove the code when ready.
It reduces indirection and allocation in the typed-RPC implementation path.
With this new implementation, there's no need to map over sequences, which
should enhance the performance of the typed implementation and clarify the code.
While this modification isn't strictly necessary for the PR introducing the
typed-RPC interface (#48), I believe it's a valuable addition. This approach aligns
with my original vision during the exploration phase, and I recommend
implementing it without significant delay. Specifically, I suggest that the
typed interface implementation for other concurrency libraries should follow a
similar path, bypassing the initial exploratory phases of building the typed-RPC
on top of the untyped one.